You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2026/08/04 23:45:37 UTC
(superset) branch master updated: fix(mcp): reject unparseable time_range instead of silently matching full table (#42283)
This is an automated email from the ASF dual-hosted git repository.
aminghadersohi 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 043163b54c0 fix(mcp): reject unparseable time_range instead of silently matching full table (#42283)
043163b54c0 is described below
commit 043163b54c0cbd7423ff9cda28ac24999c3887f5
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Tue Aug 4 19:45:30 2026 -0400
fix(mcp): reject unparseable time_range instead of silently matching full table (#42283)
Co-authored-by: Claude <no...@anthropic.com>
---
.../mcp_service/common/time_range_validation.py | 194 ++++++++++++++
superset/mcp_service/dashboard/schemas.py | 11 +
superset/mcp_service/dataset/schemas.py | 49 ++--
superset/mcp_service/semantic_layer/schemas.py | 28 ++-
.../common/test_time_range_validation.py | 280 +++++++++++++++++++++
.../dashboard/tool/test_manage_native_filters.py | 114 +++++++++
.../mcp_service/dataset/tool/test_query_dataset.py | 134 +++++++++-
.../semantic_layer/tool/test_get_table.py | 112 +++++++++
8 files changed, 886 insertions(+), 36 deletions(-)
diff --git a/superset/mcp_service/common/time_range_validation.py b/superset/mcp_service/common/time_range_validation.py
new file mode 100644
index 00000000000..f12e8ba0e1e
--- /dev/null
+++ b/superset/mcp_service/common/time_range_validation.py
@@ -0,0 +1,194 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Shared ``time_range`` validation for MCP tools that forward a free-form,
+model-generated string into a ``TEMPORAL_RANGE`` filter or a native filter's
+``default_time_range``.
+
+``superset.utils.date_parser.get_since_until()`` only rewrites a
+separator-less ``time_range`` into a bounded range when it recognizes one of
+a handful of prefixes (``Last``, ``Next``, ``previous calendar ...``,
+``Current ...``, ``first ... of ...``). Anything else -- ``banana``,
+``this month``, lowercase ``last week``, ``[decade]`` -- silently falls
+through to an unbounded ``(None, today)`` result: no error, no warning, and
+the query returns the entire table.
+
+That silent behavior is unfixable at the ``get_since_until()`` level without
+risking regressions across the whole chart/dashboard query path (see
+``jinja_context.py`` and ``semantic_layers/mapper.py``, which also call it).
+The MCP tools are the surface that accepts free-form, model-generated
+strings, so the guard lives here instead: reject anything
+``get_since_until()`` would silently discard, with a message that lists the
+accepted forms so the caller (an LLM) can self-correct.
+
+Acceptance is decided by *calling* ``get_since_until()`` and checking what
+comes back, not by re-implementing its grammar here. Mirroring the grammar
+would drift from the real parser and force a choice between rejecting valid
+freeform values (``Last Monday``, ``Last year to date``) and admitting
+malformed ones (``Last nonsense``, a bare ``Last``). Delegating gets both
+right by construction.
+"""
+
+from __future__ import annotations
+
+import re
+
+from superset.commands.chart.exceptions import (
+ TimeRangeAmbiguousError,
+ TimeRangeParseFailError,
+)
+from superset.constants import NO_TIME_RANGE
+from superset.utils.date_parser import get_since_until
+
+# Bracket shorthands (e.g. "[year]", "[quarter]") are not a Superset
+# time-range grammar -- they appear when an LLM copies a grain token from a
+# dashboard filter context. Map them to the equivalent "Last <unit>" form;
+# the sub-day ones are rewritten further by _normalize_sub_day_last() below,
+# so "[hour]" and a bare "Last hour" converge on one canonical range.
+BRACKET_SHORTHAND_TO_TIME_RANGE: dict[str, str] = {
+ "[second]": "Last second",
+ "[minute]": "Last minute",
+ "[hour]": "Last hour",
+ "[day]": "Last day",
+ "[week]": "Last week",
+ "[month]": "Last month",
+ "[quarter]": "Last quarter",
+ "[year]": "Last year",
+}
+
+_SEPARATOR = " : "
+
+_LAST_PREFIX = "Last "
+
+# The sub-day tail of a "Last ..." expression, e.g. the "5 minutes" in
+# "Last 5 minutes". Unit case is ignored because the downstream parser is
+# case-insensitive about it ("Last Hour" fails exactly like "Last hour").
+# "Next <second|minute|hour>" needs no equivalent treatment:
+# get_since_until() pairs it with a "today" (midnight) *since*, so
+# since <= until always holds.
+_SUB_DAY_REMAINDER = re.compile(
+ r"^(?:(\d{1,9})\s{1,5})?(second|minute|hour)s?$", re.IGNORECASE
+)
+
+
+def _normalize_sub_day_last(value: str) -> str | None:
+ """Rewrite a sub-day ``Last ...`` value into an explicit ``DATEADD`` range.
+
+ ``get_since_until()`` turns a separator-less ``"Last hour"`` into
+ ``"Last hour : today"``. The since side then resolves against ``now``
+ (sub-day units carry a time component) while ``today`` resolves to
+ midnight, so since lands after until and the call raises "From date
+ cannot be larger than to date". This affects every sub-day form --
+ ``Last second``, ``Last minute``, ``Last hour``, ``Last 5 minutes``,
+ ``Last 2 hours``. Anchoring both ends on ``now`` sidesteps the mismatch.
+
+ Returns ``None`` when ``value`` is not a sub-day ``Last`` expression, in
+ which case the caller falls back to ``_resolves_to_bounded_range()``.
+ """
+ if not value.startswith(_LAST_PREFIX):
+ return None
+ match = _SUB_DAY_REMAINDER.match(value[len(_LAST_PREFIX) :].strip())
+ if match is None:
+ return None
+ quantity = int(match.group(1) or 1)
+ unit = match.group(2).upper()
+ return f"DATEADD(DATETIME('now'), -{quantity}, {unit}) : DATETIME('now')"
+
+
+def _resolves_to_bounded_range(value: str) -> bool:
+ """Whether get_since_until() turns this separator-less value into a
+ range bounded on both ends.
+
+ Asks the real parser instead of re-implementing its grammar, which is
+ what makes the accept/reject split exact:
+
+ * ``Last 7 days``, ``Current week``, ``previous calendar year``,
+ ``first week of this year`` -- and freeform tails the parser handles
+ on its own, like ``Last Monday``, ``Last January``,
+ ``Last year to date`` -- all resolve, so they're accepted.
+ * ``banana``, ``this month``, lowercase ``last week`` resolve to an
+ unbounded ``(None, today)``: the silent full-table match this module
+ exists to catch.
+ * ``Last nonsense``, ``Last decade`` and a bare ``Last`` raise inside
+ the parser. Rejecting them here turns what would surface as a
+ low-level parse error deep in the query path into a field-level
+ ``ValidationError`` carrying the accepted-format guidance.
+
+ Sub-day ``Last`` values must be rewritten by ``_normalize_sub_day_last()``
+ before reaching this check -- they raise here, but the fix is to
+ normalize them, not to reject them.
+ """
+ try:
+ since, until = get_since_until(time_range=value)
+ except (ValueError, TimeRangeParseFailError, TimeRangeAmbiguousError):
+ return False
+ return since is not None and until is not None
+
+
+def validate_time_range(value: str | None) -> str | None:
+ """Normalize and validate an MCP ``time_range`` / ``default_time_range``
+ string.
+
+ Returns the canonicalized value. Raises ``ValueError`` -- which Pydantic
+ converts into a field ``ValidationError`` -- when ``value`` is a bare
+ (non-range) string that ``get_since_until()`` cannot resolve to a
+ bounded range. Such values previously passed straight through and
+ silently produced an unfiltered, full-table match.
+ """
+ if value is None:
+ return None
+
+ stripped = value.strip()
+ # An empty string is the "no default" / "no filter applied" sentinel
+ # for these tools (callers gate on truthiness before using it), not a
+ # value get_since_until() ever sees -- pass it through unchanged.
+ if not stripped:
+ return stripped
+
+ if stripped == NO_TIME_RANGE:
+ return stripped
+
+ if (canonical := BRACKET_SHORTHAND_TO_TIME_RANGE.get(stripped.lower())) is not None:
+ stripped = canonical
+
+ if (rewritten := _normalize_sub_day_last(stripped)) is not None:
+ return rewritten
+
+ # A value carrying the separator is already an explicit "<start> : <end>"
+ # range, so it can't hit the silent unbounded fallback. Any parse failure
+ # in either half raises loudly downstream, which is a signal the caller
+ # can act on -- leave those to the parser rather than second-guessing
+ # them here, where the relative anchors may differ from the query's.
+ if _SEPARATOR in stripped:
+ return stripped
+
+ if _resolves_to_bounded_range(stripped):
+ return stripped
+
+ raise ValueError(
+ f"Unrecognized time_range value: {value!r}. A bare (non-range) "
+ "time_range must be one of: 'Last <unit>' (e.g. 'Last 7 days', "
+ "'Last month', 'Last year', 'Last hour', 'Last 5 minutes'), "
+ "'Next <unit>', 'previous calendar "
+ "<week|month|quarter|year>', 'Current <day|week|month|quarter|"
+ "year>', 'first <week|month|quarter> of [this|last|next|prior] "
+ "<week|month|quarter|year>', or a bracket shorthand like "
+ f"{sorted(BRACKET_SHORTHAND_TO_TIME_RANGE)}. For anything else, "
+ "use an explicit '<start> : <end>' range, e.g. "
+ "'2024-01-01 : 2024-12-31'."
+ )
diff --git a/superset/mcp_service/dashboard/schemas.py b/superset/mcp_service/dashboard/schemas.py
index 2d5586e9035..18fae25d94a 100644
--- a/superset/mcp_service/dashboard/schemas.py
+++ b/superset/mcp_service/dashboard/schemas.py
@@ -94,6 +94,7 @@ from superset.mcp_service.common.pagination_schemas import (
PaginatedListRequest,
PaginatedResponse,
)
+from superset.mcp_service.common.time_range_validation import validate_time_range
from superset.mcp_service.privacy import (
filter_user_directory_fields,
strip_user_directory_fields_from_schema,
@@ -2228,6 +2229,11 @@ class FilterTimeSpec(BaseNewFilterSpec):
),
)
+ @field_validator("default_time_range")
+ @classmethod
+ def _validate_default_time_range(cls, v: str | None) -> str | None:
+ return validate_time_range(v)
+
NewNativeFilterSpec = Annotated[
FilterSelectSpec | FilterTimeSpec,
@@ -2279,6 +2285,11 @@ class NativeFilterUpdateSpec(BaseModel):
),
)
+ @field_validator("default_time_range")
+ @classmethod
+ def _validate_default_time_range(cls, v: str | None) -> str | None:
+ return validate_time_range(v)
+
class ManageNativeFiltersRequest(BaseModel):
"""Request schema for the manage_native_filters tool."""
diff --git a/superset/mcp_service/dataset/schemas.py b/superset/mcp_service/dataset/schemas.py
index 32a8214b865..67b4e22a92b 100644
--- a/superset/mcp_service/dataset/schemas.py
+++ b/superset/mcp_service/dataset/schemas.py
@@ -47,6 +47,7 @@ from superset.mcp_service.common.pagination_schemas import (
PaginatedListRequest,
PaginatedResponse,
)
+from superset.mcp_service.common.time_range_validation import validate_time_range
from superset.mcp_service.privacy import (
filter_user_directory_fields,
strip_user_directory_fields_from_schema,
@@ -736,28 +737,19 @@ class QueryDatasetFilter(BaseModel):
description="Filter value (omit for IS NULL/IS NOT NULL)",
)
-
-# Bracket shorthands (e.g. "[year]", "[quarter]") are not a Superset
-# time-range grammar — they appear when an LLM copies a grain token from a
-# dashboard filter context. Map them to an equivalent form that
-# get_since_until() resolves correctly.
-#
-# "Last second"/"Last minute"/"Last hour" are excluded: get_since_until()
-# pairs a "Last <unit>" since-expression (resolved against "now" for
-# sub-day units) with a default until-expression resolved against "today"
-# (midnight), so since ends up after until and raises "From date cannot
-# be larger than to date". Explicit DATEADD/DATETIME expressions sidestep
-# that mismatch by resolving both ends against "now".
-_BRACKET_SHORTHAND_TO_TIME_RANGE: dict[str, str] = {
- "[second]": "DATEADD(DATETIME('now'), -1, SECOND) : DATETIME('now')",
- "[minute]": "DATEADD(DATETIME('now'), -1, MINUTE) : DATETIME('now')",
- "[hour]": "DATEADD(DATETIME('now'), -1, HOUR) : DATETIME('now')",
- "[day]": "Last day",
- "[week]": "Last week",
- "[month]": "Last month",
- "[quarter]": "Last quarter",
- "[year]": "Last year",
-}
+ @model_validator(mode="after")
+ def _validate_temporal_range_val(self) -> "QueryDatasetFilter":
+ """Hold a TEMPORAL_RANGE filter to the same grammar as ``time_range``.
+
+ This operator resolves through ``get_since_until()`` exactly like the
+ dedicated ``time_range`` field does, so an unparseable value here
+ produces the same silent full-table match. Validating only
+ ``time_range`` would leave that gap open to any caller that spells
+ the same filter out longhand.
+ """
+ if self.op == "TEMPORAL_RANGE" and isinstance(self.val, str):
+ self.val = validate_time_range(self.val)
+ return self
class QueryDatasetRequest(QueryCacheControl):
@@ -795,8 +787,9 @@ class QueryDatasetRequest(QueryCacheControl):
"'Last 7 days', 'Last month', 'Last year', 'Last quarter', "
"'Current week', 'previous calendar year', or an ISO-8601 range "
"like '2024-01-01 : 2024-12-31'. Requires a temporal column "
- "on the dataset. Do NOT use bracket shorthands like '[year]' "
- "or '[quarter]' — use 'Last year' / 'Last quarter' instead."
+ "on the dataset. Bracket shorthands like '[year]' or "
+ "'[quarter]' are also accepted and normalized to the "
+ "equivalent 'Last <unit>' form."
),
)
time_column: str | None = Field(
@@ -823,12 +816,8 @@ class QueryDatasetRequest(QueryCacheControl):
@field_validator("time_range")
@classmethod
- def normalize_time_range(cls, v: str | None) -> str | None:
- if v is None:
- return v
- stripped = v.strip()
- canonical = _BRACKET_SHORTHAND_TO_TIME_RANGE.get(stripped.lower())
- return canonical if canonical is not None else stripped
+ def _validate_time_range(cls, v: str | None) -> str | None:
+ return validate_time_range(v)
@model_validator(mode="after")
def validate_metrics_or_columns(self) -> "QueryDatasetRequest":
diff --git a/superset/mcp_service/semantic_layer/schemas.py b/superset/mcp_service/semantic_layer/schemas.py
index a031b28385a..6dfceeb6069 100644
--- a/superset/mcp_service/semantic_layer/schemas.py
+++ b/superset/mcp_service/semantic_layer/schemas.py
@@ -21,11 +21,12 @@ from __future__ import annotations
from typing import Any, Literal
-from pydantic import BaseModel, Field
+from pydantic import BaseModel, Field, field_validator, model_validator
from superset.mcp_service.chart.schemas import DataColumn, PerformanceMetadata
from superset.mcp_service.common.cache_schemas import CacheStatus
from superset.mcp_service.common.error_schemas import MCPBaseError
+from superset.mcp_service.common.time_range_validation import validate_time_range
# ---------------------------------------------------------------------------
# Shared error schema
@@ -147,6 +148,18 @@ class GetTableFilter(BaseModel):
description="Filter value. Use a list for 'IN'/'NOT IN' operators.",
)
+ @model_validator(mode="after")
+ def _validate_temporal_range_val(self) -> "GetTableFilter":
+ """Hold a TEMPORAL_RANGE filter to the same grammar as ``time_range``.
+
+ This operator resolves through ``get_since_until()`` exactly like the
+ dedicated ``time_range`` field does, so an unparseable value here
+ produces the same silent full-table match.
+ """
+ if self.op == "TEMPORAL_RANGE" and isinstance(self.val, str):
+ self.val = validate_time_range(self.val)
+ return self
+
class GetTableRequest(BaseModel):
"""Request schema for get_table."""
@@ -183,8 +196,12 @@ class GetTableRequest(BaseModel):
time_range: str | None = Field(
default=None,
description=(
- "Optional time range string, e.g. 'Last 7 days', 'Last 30 days', "
- "'2024-01-01 : 2024-12-31'. Requires a datetime dimension."
+ "Optional time range string. Use Superset relative shorthands "
+ "like 'Last 7 days', 'Last 30 days', 'Last year', 'Current "
+ "week', 'previous calendar year', or an ISO-8601 range like "
+ "'2024-01-01 : 2024-12-31'. Requires a datetime dimension. "
+ "Bracket shorthands like '[year]' or '[quarter]' are also "
+ "accepted and normalized to the equivalent 'Last <unit>' form."
),
)
time_column: str | None = Field(
@@ -214,6 +231,11 @@ class GetTableRequest(BaseModel):
description="Force a cache refresh even when cached results exist.",
)
+ @field_validator("time_range")
+ @classmethod
+ def _validate_time_range(cls, v: str | None) -> str | None:
+ return validate_time_range(v)
+
class GetTableResponse(BaseModel):
"""Response schema for get_table."""
diff --git a/tests/unit_tests/mcp_service/common/test_time_range_validation.py b/tests/unit_tests/mcp_service/common/test_time_range_validation.py
new file mode 100644
index 00000000000..8dae0e4ca51
--- /dev/null
+++ b/tests/unit_tests/mcp_service/common/test_time_range_validation.py
@@ -0,0 +1,280 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Unit tests for the shared MCP time_range validator.
+
+These cases mirror the live-testing findings from SC-114824: values that
+superset.utils.date_parser.get_since_until() silently resolves to an
+unbounded (None, today) range -- rather than raising -- must be rejected
+here instead of reaching that function.
+"""
+
+from __future__ import annotations
+
+import pytest
+
+from superset.commands.chart.exceptions import TimeRangeParseFailError
+from superset.mcp_service.common.time_range_validation import (
+ BRACKET_SHORTHAND_TO_TIME_RANGE,
+ validate_time_range,
+)
+from superset.utils.date_parser import get_since_until
+
+
+class TestValidateTimeRangePassthrough:
+ """Values get_since_until() already resolves correctly pass through
+ unchanged (aside from whitespace trimming)."""
+
+ def test_none_passes_through(self) -> None:
+ assert validate_time_range(None) is None
+
+ def test_empty_string_passes_through(self) -> None:
+ assert validate_time_range("") == ""
+
+ def test_whitespace_only_becomes_empty(self) -> None:
+ assert validate_time_range(" ") == ""
+
+ def test_no_filter_sentinel_passes_through(self) -> None:
+ assert validate_time_range("No filter") == "No filter"
+
+ @pytest.mark.parametrize(
+ "value",
+ [
+ "Last 7 days",
+ "Last month",
+ "Last year",
+ "Last quarter",
+ "Last week",
+ "Next 5 days",
+ "Next month",
+ "previous calendar week",
+ "previous calendar month",
+ "previous calendar quarter",
+ "previous calendar year",
+ "Current day",
+ "Current week",
+ "Current month",
+ "Current quarter",
+ "Current year",
+ "first week of this year",
+ "first month of this quarter",
+ "first week of last month",
+ ],
+ )
+ def test_recognized_bare_prefix_unchanged(self, value: str) -> None:
+ assert validate_time_range(value) == value
+ # Cross-check against the real parser: every accepted value must
+ # actually resolve to a bounded range, not just avoid raising here.
+ since, until = get_since_until(time_range=value)
+ assert since is not None
+ assert until is not None
+
+ @pytest.mark.parametrize(
+ "value",
+ [
+ "2024-01-01 : 2024-12-31",
+ "2003-01-01 : 2004-01-01",
+ "yesterday : tomorrow",
+ "banana : split", # has a separator; loud parse errors happen downstream
+ ],
+ )
+ def test_separator_containing_value_unchanged(self, value: str) -> None:
+ assert validate_time_range(value) == value
+
+ def test_strips_surrounding_whitespace(self) -> None:
+ assert validate_time_range(" Last 7 days ") == "Last 7 days"
+
+
+class TestValidateTimeRangeSubDayLast:
+ """Sub-day ``Last ...`` values are rewritten to an explicit DATEADD range.
+
+ get_since_until() pairs a sub-day since-expression (resolved against
+ ``now``) with a default until of ``today`` (midnight), so since lands
+ after until and it raises "From date cannot be larger than to date".
+ Anchoring both ends on ``now`` fixes that.
+ """
+
+ @pytest.mark.parametrize(
+ "value,expected",
+ [
+ ("Last second", "DATEADD(DATETIME('now'), -1, SECOND) : DATETIME('now')"),
+ ("Last minute", "DATEADD(DATETIME('now'), -1, MINUTE) : DATETIME('now')"),
+ ("Last hour", "DATEADD(DATETIME('now'), -1, HOUR) : DATETIME('now')"),
+ (
+ "Last 30 seconds",
+ "DATEADD(DATETIME('now'), -30, SECOND) : DATETIME('now')",
+ ),
+ (
+ "Last 5 minutes",
+ "DATEADD(DATETIME('now'), -5, MINUTE) : DATETIME('now')",
+ ),
+ ("Last 2 hours", "DATEADD(DATETIME('now'), -2, HOUR) : DATETIME('now')"),
+ ("Last 1 hour", "DATEADD(DATETIME('now'), -1, HOUR) : DATETIME('now')"),
+ # Unit casing is ignored -- the downstream parser is
+ # case-insensitive about it, so "Last Hour" fails identically.
+ ("Last Hour", "DATEADD(DATETIME('now'), -1, HOUR) : DATETIME('now')"),
+ ],
+ )
+ def test_sub_day_last_normalizes(self, value: str, expected: str) -> None:
+ # Confirm the premise: the raw value really does blow up downstream.
+ with pytest.raises(ValueError, match="From date cannot be larger"):
+ get_since_until(time_range=value)
+
+ result = validate_time_range(value)
+ assert result == expected
+ since, until = get_since_until(time_range=result)
+ assert since is not None
+ assert until is not None
+ assert since < until
+
+ @pytest.mark.parametrize(
+ "value",
+ [
+ "Last day",
+ "Last week",
+ "Last 7 days",
+ "Last month",
+ "Last quarter",
+ "Last year",
+ # Freeform tails get_since_until() resolves via its own parser --
+ # a unit whitelist would wrongly reject these.
+ "Last Monday",
+ "Last January",
+ "Last year to date",
+ "Last 3 days ago",
+ ],
+ )
+ def test_day_and_coarser_last_values_untouched(self, value: str) -> None:
+ assert validate_time_range(value) == value
+ since, until = get_since_until(time_range=value)
+ assert since is not None
+ assert until is not None
+ assert since < until
+
+ def test_lowercase_last_sub_day_not_normalized(self) -> None:
+ """The "Last" prefix stays case-sensitive, mirroring
+ get_since_until()'s own check -- "last hour" doesn't match its
+ rewrite either, so it's an unrecognized bare value, not a
+ normalization target."""
+ with pytest.raises(ValueError, match="Unrecognized time_range"):
+ validate_time_range("last hour")
+
+ def test_next_sub_day_passes_through_unchanged(self) -> None:
+ """ "Next <sub-day unit>" doesn't hit the since/until mismatch --
+ get_since_until() pairs it with a "today" (midnight) since, which
+ is always <= the "now"-based until -- so it needs no normalization."""
+ assert validate_time_range("Next hour") == "Next hour"
+ since, until = get_since_until(time_range="Next hour")
+ assert since is not None
+ assert until is not None
+ assert since < until
+
+
+class TestValidateTimeRangeBracketShorthand:
+ """Bracket shorthands (from apache/superset#42144) still auto-correct."""
+
+ @pytest.mark.parametrize("bracket", sorted(BRACKET_SHORTHAND_TO_TIME_RANGE))
+ def test_bracket_shorthand_normalizes(self, bracket: str) -> None:
+ result = validate_time_range(bracket)
+ # Sub-day shorthands route through the DATEADD rewrite; the rest
+ # resolve as the plain "Last <unit>" they map to.
+ assert result == validate_time_range(BRACKET_SHORTHAND_TO_TIME_RANGE[bracket])
+ since, until = get_since_until(time_range=result)
+ assert since is not None
+ assert until is not None
+ assert since < until
+
+ def test_bracket_shorthand_case_insensitive(self) -> None:
+ assert validate_time_range("[YEAR]") == "Last year"
+
+ def test_sub_day_bracket_shorthand_matches_bare_form(self) -> None:
+ """'[hour]' and 'Last hour' converge on the same canonical range."""
+ assert validate_time_range("[hour]") == validate_time_range("Last hour")
+
+ def test_bracket_shorthand_whitespace_tolerant(self) -> None:
+ assert validate_time_range(" [year] ") == "Last year"
+
+ def test_unrecognized_bracket_shorthand_rejected(self) -> None:
+ """'[decade]' is not one of the eight recognized grain tokens."""
+ with pytest.raises(ValueError, match="Unrecognized time_range"):
+ validate_time_range("[decade]")
+
+
+class TestValidateTimeRangeRejectsSilentFailures:
+ """Regression guard: values that silently produced a full-table match
+ (SC-114824) must now raise instead."""
+
+ @pytest.mark.parametrize(
+ "value",
+ [
+ "banana",
+ "this week",
+ "this month",
+ "last week", # lowercase -- get_since_until requires "Last"
+ "yesterday",
+ "[decade]",
+ ],
+ )
+ def test_previously_silent_values_now_raise(self, value: str) -> None:
+ # Confirm the premise: get_since_until() really does silently
+ # discard this value (unbounded start, no exception) before
+ # asserting our validator closes the gap.
+ since, until = get_since_until(time_range=value)
+ assert since is None
+
+ with pytest.raises(ValueError, match="Unrecognized time_range"):
+ validate_time_range(value)
+
+
+class TestValidateTimeRangeRejectsMalformedPrefixes:
+ """A recognized prefix is not enough -- the value has to actually parse.
+
+ These start with "Last"/"Next" but blow up inside get_since_until(),
+ so they'd otherwise surface as a low-level parse error deep in the
+ query path instead of a field-level ValidationError the caller can act
+ on.
+ """
+
+ @pytest.mark.parametrize(
+ "value",
+ [
+ "Last nonsense",
+ "Next nonsense",
+ "Last",
+ "Next",
+ "Lastly",
+ "Lastminute",
+ "Last decade",
+ "Last fortnight",
+ ],
+ )
+ def test_unparseable_prefixed_values_rejected(self, value: str) -> None:
+ # Confirm the premise: the raw value doesn't survive the parser.
+ with pytest.raises(TimeRangeParseFailError):
+ get_since_until(time_range=value)
+
+ with pytest.raises(ValueError, match="Unrecognized time_range"):
+ validate_time_range(value)
+
+ def test_prefix_lookalike_that_silently_matches_is_rejected(self) -> None:
+ """'Lasagna' starts with neither prefix but shares 'Las' -- it takes
+ the silent unbounded path, not the raising one."""
+ since, _ = get_since_until(time_range="Lasagna")
+ assert since is None
+
+ with pytest.raises(ValueError, match="Unrecognized time_range"):
+ validate_time_range("Lasagna")
diff --git a/tests/unit_tests/mcp_service/dashboard/tool/test_manage_native_filters.py b/tests/unit_tests/mcp_service/dashboard/tool/test_manage_native_filters.py
index bbf9a92b6be..c71f90d1241 100644
--- a/tests/unit_tests/mcp_service/dashboard/tool/test_manage_native_filters.py
+++ b/tests/unit_tests/mcp_service/dashboard/tool/test_manage_native_filters.py
@@ -816,3 +816,117 @@ async def test_dashboard_forbidden(mcp_server):
assert data["permission_denied"] is True
assert "permission" in data["error"]
+
+
+# ---------------------------------------------------------------------------
+# time_range validation (SC-114824)
+# ---------------------------------------------------------------------------
+
+
+class TestFilterTimeSpecTimeRangeValidation:
+ """FilterTimeSpec.default_time_range rejects values get_since_until()
+ would otherwise silently resolve to an unbounded, full-table range
+ (e.g. baking a dead default into a saved dashboard filter config).
+
+ See SC-114824: shared validator in
+ superset.mcp_service.common.time_range_validation.
+ """
+
+ def test_valid_relative_range_passes(self) -> None:
+ from superset.mcp_service.dashboard.schemas import FilterTimeSpec
+
+ spec = FilterTimeSpec.model_validate(
+ {
+ "filter_type": "filter_time",
+ "name": "Time Range",
+ "default_time_range": "Last week",
+ }
+ )
+ assert spec.default_time_range == "Last week"
+
+ def test_bracket_shorthand_normalizes(self) -> None:
+ from superset.mcp_service.dashboard.schemas import FilterTimeSpec
+
+ spec = FilterTimeSpec.model_validate(
+ {
+ "filter_type": "filter_time",
+ "name": "Time Range",
+ "default_time_range": "[year]",
+ }
+ )
+ assert spec.default_time_range == "Last year"
+
+ def test_omitted_default_passes(self) -> None:
+ from superset.mcp_service.dashboard.schemas import FilterTimeSpec
+
+ spec = FilterTimeSpec.model_validate(
+ {"filter_type": "filter_time", "name": "Time Range"}
+ )
+ assert spec.default_time_range is None
+
+ @pytest.mark.parametrize("bad_value", ["banana", "[year", "this month"])
+ def test_previously_silent_values_now_raise(self, bad_value: str) -> None:
+ from pydantic import ValidationError
+
+ from superset.mcp_service.dashboard.schemas import FilterTimeSpec
+
+ with pytest.raises(ValidationError, match="Unrecognized time_range"):
+ FilterTimeSpec.model_validate(
+ {
+ "filter_type": "filter_time",
+ "name": "Time Range",
+ "default_time_range": bad_value,
+ }
+ )
+
+
+class TestNativeFilterUpdateSpecTimeRangeValidation:
+ """NativeFilterUpdateSpec.default_time_range gets the same guard."""
+
+ def test_valid_relative_range_passes(self) -> None:
+ from superset.mcp_service.dashboard.schemas import NativeFilterUpdateSpec
+
+ spec = NativeFilterUpdateSpec.model_validate(
+ {"id": "NATIVE_FILTER-1", "default_time_range": "Last month"}
+ )
+ assert spec.default_time_range == "Last month"
+
+ def test_previously_silent_value_now_raises(self) -> None:
+ from pydantic import ValidationError
+
+ from superset.mcp_service.dashboard.schemas import NativeFilterUpdateSpec
+
+ with pytest.raises(ValidationError, match="Unrecognized time_range"):
+ NativeFilterUpdateSpec.model_validate(
+ {"id": "NATIVE_FILTER-1", "default_time_range": "this week"}
+ )
+
+
+@pytest.mark.asyncio
+async def test_add_filter_time_rejects_unparseable_default(mcp_server):
+ """End-to-end: manage_native_filters must not persist a dead time
+ filter default into the dashboard's saved filter config.
+
+ Request-schema validation errors (like an unrecognized time_range)
+ happen at the MCP tool-call boundary, before the tool body runs, so
+ the client raises ToolError rather than returning a JSON error body.
+ """
+ from fastmcp.exceptions import ToolError
+
+ dashboard = _mock_dashboard(filters=[])
+
+ with patch(DAO_FIND_BY_ID, return_value=dashboard):
+ with pytest.raises(ToolError, match="Unrecognized time_range"):
+ await _call(
+ mcp_server,
+ {
+ "dashboard_id": 1,
+ "add": [
+ {
+ "filter_type": "filter_time",
+ "name": "Time Range",
+ "default_time_range": "this week",
+ }
+ ],
+ },
+ )
diff --git a/tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py b/tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py
index 4f453c94f0a..9f66d01a0f0 100644
--- a/tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py
+++ b/tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py
@@ -950,10 +950,12 @@ class TestQueryDatasetBracketShorthandNormalization:
def test_hour_bracket_normalized(self) -> None:
"""'[hour]' maps to an explicit DATEADD/DATETIME expression.
- 'Last hour' is deliberately not used: get_since_until() resolves its
- since-expression against 'now' but its default until-expression
+ A bare 'Last hour' can't be used as-is: get_since_until() resolves
+ its since-expression against 'now' but its default until-expression
against 'today' (midnight), so since ends up after until and raises
- a "From date cannot be larger than to date" error.
+ a "From date cannot be larger than to date" error. The validator
+ applies the same rewrite to both spellings -- see
+ test_bare_sub_day_last_normalized below.
"""
from superset.mcp_service.dataset.schemas import QueryDatasetRequest
@@ -982,6 +984,34 @@ class TestQueryDatasetBracketShorthandNormalization:
req.time_range == "DATEADD(DATETIME('now'), -1, SECOND) : DATETIME('now')"
)
+ @pytest.mark.parametrize(
+ "value,expected",
+ [
+ ("Last hour", "DATEADD(DATETIME('now'), -1, HOUR) : DATETIME('now')"),
+ (
+ "Last 15 minutes",
+ "DATEADD(DATETIME('now'), -15, MINUTE) : DATETIME('now')",
+ ),
+ (
+ "Last 30 seconds",
+ "DATEADD(DATETIME('now'), -30, SECOND) : DATETIME('now')",
+ ),
+ ],
+ )
+ def test_bare_sub_day_last_normalized(self, value: str, expected: str) -> None:
+ """Sub-day 'Last ...' gets the same rewrite as its bracket form.
+
+ Without it these reach get_since_until() as-is and raise "From date
+ cannot be larger than to date" from deep in the query path, rather
+ than being resolved to the range the caller asked for.
+ """
+ from superset.mcp_service.dataset.schemas import QueryDatasetRequest
+
+ req = QueryDatasetRequest.model_validate(
+ {"dataset_id": 1, "metrics": ["count"], "time_range": value}
+ )
+ assert req.time_range == expected
+
def test_bracket_uppercase_normalized(self) -> None:
from superset.mcp_service.dataset.schemas import QueryDatasetRequest
@@ -1070,6 +1100,104 @@ class TestQueryDatasetBracketShorthandNormalization:
assert req.time_range is None
+class TestQueryDatasetTimeRangeValidation:
+ """QueryDatasetRequest.time_range rejects values get_since_until()
+ would otherwise silently resolve to an unbounded, full-table range.
+
+ See SC-114824: shared validator in
+ superset.mcp_service.common.time_range_validation. Complements
+ TestQueryDatasetBracketShorthandNormalization above, which only
+ covers the eight recognized bracket tokens -- this class covers the
+ open class of previously-silent, non-bracket values the shared
+ validator now rejects.
+ """
+
+ @pytest.mark.parametrize(
+ "bad_value",
+ ["banana", "this week", "this month", "last week", "yesterday", "[decade]"],
+ )
+ def test_previously_silent_values_now_raise(self, bad_value: str) -> None:
+ """These values used to silently return an unfiltered, full-table
+ result (empty warnings, success: true). They must now raise a
+ ValidationError instead."""
+ from pydantic import ValidationError
+
+ from superset.mcp_service.dataset.schemas import QueryDatasetRequest
+
+ with pytest.raises(ValidationError, match="Unrecognized time_range"):
+ QueryDatasetRequest.model_validate(
+ {"dataset_id": 1, "metrics": ["count"], "time_range": bad_value}
+ )
+
+
+class TestQueryDatasetTemporalRangeFilterValidation:
+ """A TEMPORAL_RANGE spelled out longhand in `filters` gets the same
+ validation as the dedicated `time_range` field.
+
+ query_dataset forwards request.filters into the query verbatim, and
+ TEMPORAL_RANGE values resolve through get_since_until() just like
+ time_range does -- so validating only time_range would leave the
+ identical silent full-table match reachable through this field.
+ """
+
+ @staticmethod
+ def _request(val: Any) -> dict[str, Any]:
+ return {
+ "dataset_id": 1,
+ "metrics": ["count"],
+ "filters": [{"col": "ts", "op": "TEMPORAL_RANGE", "val": val}],
+ }
+
+ @pytest.mark.parametrize(
+ "bad_value",
+ ["banana", "this month", "last week", "[decade]", "Last nonsense"],
+ )
+ def test_malformed_temporal_range_filter_rejected(self, bad_value: str) -> None:
+ from pydantic import ValidationError
+
+ from superset.mcp_service.dataset.schemas import QueryDatasetRequest
+
+ with pytest.raises(ValidationError, match="Unrecognized time_range"):
+ QueryDatasetRequest.model_validate(self._request(bad_value))
+
+ def test_temporal_range_filter_normalizes_like_time_range(self) -> None:
+ from superset.mcp_service.dataset.schemas import QueryDatasetRequest
+
+ req = QueryDatasetRequest.model_validate(self._request("Last hour"))
+ assert req.filters[0].val == (
+ "DATEADD(DATETIME('now'), -1, HOUR) : DATETIME('now')"
+ )
+
+ @pytest.mark.parametrize("good_value", ["Last 7 days", "2024-01-01 : 2024-12-31"])
+ def test_valid_temporal_range_filter_unchanged(self, good_value: str) -> None:
+ from superset.mcp_service.dataset.schemas import QueryDatasetRequest
+
+ req = QueryDatasetRequest.model_validate(self._request(good_value))
+ assert req.filters[0].val == good_value
+
+ def test_non_temporal_operator_value_untouched(self) -> None:
+ """Only TEMPORAL_RANGE goes through the time grammar -- 'banana' is
+ a perfectly good value to compare a text column against."""
+ from superset.mcp_service.dataset.schemas import QueryDatasetRequest
+
+ req = QueryDatasetRequest.model_validate(
+ {
+ "dataset_id": 1,
+ "metrics": ["count"],
+ "filters": [{"col": "fruit", "op": "==", "val": "banana"}],
+ }
+ )
+ assert req.filters[0].val == "banana"
+
+ def test_non_string_temporal_value_left_to_downstream(self) -> None:
+ """A non-string val isn't a time_range expression at all; the time
+ grammar has nothing to say about it."""
+ from superset.mcp_service.dataset.schemas import QueryDatasetRequest
+
+ req = QueryDatasetRequest.model_validate(self._request(None))
+ assert req.filters[0].val is None
+
+
@pytest.mark.asyncio
async def test_query_dataset_bracket_year_resolves_without_parse_error(
mcp_server: FastMCP,
diff --git a/tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py b/tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py
index f5725bdc3fa..a7cfe3f4d67 100644
--- a/tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py
+++ b/tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py
@@ -526,3 +526,115 @@ async def test_get_table_builtin_time_range_without_configured_dttm_validation_e
assert data["success"] is False
assert data["error_type"] == "ValidationError"
assert "no temporal column is configured" in data["message"]
+
+
+class TestGetTableTimeRangeValidation:
+ """GetTableRequest.time_range rejects values get_since_until() would
+ otherwise silently resolve to an unbounded, full-table range.
+
+ See SC-114824: shared validator in
+ superset.mcp_service.common.time_range_validation.
+ """
+
+ def test_valid_relative_range_passes(self) -> None:
+ from superset.mcp_service.semantic_layer.schemas import GetTableRequest
+
+ req = GetTableRequest.model_validate(
+ {"dataset_id": 1, "metrics": ["count"], "time_range": "Last 30 days"}
+ )
+ assert req.time_range == "Last 30 days"
+
+ def test_iso_range_passes(self) -> None:
+ from superset.mcp_service.semantic_layer.schemas import GetTableRequest
+
+ req = GetTableRequest.model_validate(
+ {
+ "dataset_id": 1,
+ "metrics": ["count"],
+ "time_range": "2003-01-01 : 2004-01-01",
+ }
+ )
+ assert req.time_range == "2003-01-01 : 2004-01-01"
+
+ def test_bracket_shorthand_normalizes(self) -> None:
+ from superset.mcp_service.semantic_layer.schemas import GetTableRequest
+
+ req = GetTableRequest.model_validate(
+ {"dataset_id": 1, "metrics": ["count"], "time_range": "[quarter]"}
+ )
+ assert req.time_range == "Last quarter"
+
+ @pytest.mark.parametrize(
+ "bad_value",
+ ["banana", "this week", "this month", "last week", "yesterday", "[decade]"],
+ )
+ def test_previously_silent_values_now_raise(self, bad_value: str) -> None:
+ """Live testing against dataset 28 (cleaned_sales_data, 2823 rows)
+ showed these values returned the entire table with success: true
+ and empty warnings. They must now raise a ValidationError."""
+ from pydantic import ValidationError
+
+ from superset.mcp_service.semantic_layer.schemas import GetTableRequest
+
+ with pytest.raises(ValidationError, match="Unrecognized time_range"):
+ GetTableRequest.model_validate(
+ {"dataset_id": 1, "metrics": ["count"], "time_range": bad_value}
+ )
+
+ def test_none_passes(self) -> None:
+ from superset.mcp_service.semantic_layer.schemas import GetTableRequest
+
+ req = GetTableRequest.model_validate(
+ {"dataset_id": 1, "metrics": ["count"], "time_range": None}
+ )
+ assert req.time_range is None
+
+
+class TestGetTableTemporalRangeFilterValidation:
+ """A TEMPORAL_RANGE spelled out longhand in `filters` gets the same
+ validation as the dedicated `time_range` field -- otherwise the
+ identical silent full-table match stays reachable through that field.
+ """
+
+ @staticmethod
+ def _request(val: Any) -> dict[str, Any]:
+ return {
+ "dataset_id": 1,
+ "metrics": ["count"],
+ "filters": [{"col": "ts", "op": "TEMPORAL_RANGE", "val": val}],
+ }
+
+ @pytest.mark.parametrize("bad_value", ["banana", "this month", "Last nonsense"])
+ def test_malformed_temporal_range_filter_rejected(self, bad_value: str) -> None:
+ from pydantic import ValidationError
+
+ from superset.mcp_service.semantic_layer.schemas import GetTableRequest
+
+ with pytest.raises(ValidationError, match="Unrecognized time_range"):
+ GetTableRequest.model_validate(self._request(bad_value))
+
+ def test_temporal_range_filter_normalizes_like_time_range(self) -> None:
+ from superset.mcp_service.semantic_layer.schemas import GetTableRequest
+
+ req = GetTableRequest.model_validate(self._request("Last hour"))
+ assert req.filters[0].val == (
+ "DATEADD(DATETIME('now'), -1, HOUR) : DATETIME('now')"
+ )
+
+ def test_valid_temporal_range_filter_unchanged(self) -> None:
+ from superset.mcp_service.semantic_layer.schemas import GetTableRequest
+
+ req = GetTableRequest.model_validate(self._request("Last 7 days"))
+ assert req.filters[0].val == "Last 7 days"
+
+ def test_non_temporal_operator_value_untouched(self) -> None:
+ from superset.mcp_service.semantic_layer.schemas import GetTableRequest
+
+ req = GetTableRequest.model_validate(
+ {
+ "dataset_id": 1,
+ "metrics": ["count"],
+ "filters": [{"col": "fruit", "op": "==", "val": "banana"}],
+ }
+ )
+ assert req.filters[0].val == "banana"