aminghadersohi commented on code in PR #39915:
URL: https://github.com/apache/superset/pull/39915#discussion_r3236189215
##########
tests/unit_tests/mcp_service/chart/test_chart_schemas.py:
##########
@@ -778,3 +778,137 @@ def
test_client_warnings_discarded_even_when_server_also_warns(self) -> None:
assert len(req.sanitization_warnings) == 1
assert "chart_name" in req.sanitization_warnings[0]
assert "injected" not in req.sanitization_warnings[0]
+
+
+class TestColumnRefNameRelaxedPattern:
+ """ColumnRef.name no longer enforces a strict regex pattern.
+
+ Many valid database column names were previously rejected:
+ - Names starting with a digit (e.g. "1Q_revenue")
+ - Names with locale-specific characters
+ The field_validator sanitize_name() still blocks XSS and SQL injection.
+ """
+
+ def test_digit_prefixed_name_accepted(self) -> None:
+ """Column names starting with a digit must now be accepted."""
+ col = ColumnRef(name="1Q_revenue")
+ assert col.name == "1Q_revenue"
+
+ def test_name_with_hyphen_accepted(self) -> None:
+ col = ColumnRef(name="order-date")
+ assert col.name == "order-date"
+
+ def test_name_with_dot_accepted(self) -> None:
+ col = ColumnRef(name="schema.column")
+ assert col.name == "schema.column"
+
+ def test_name_with_spaces_accepted(self) -> None:
+ col = ColumnRef(name="Total Revenue")
+ assert col.name == "Total Revenue"
+
+ def test_script_tag_neutralized(self) -> None:
+ """sanitize_name() neutralizes script-tag XSS via nh3.
+ Depending on nh3 version, nh3 either strips the entire script element
+ including its content (leaving empty → ValidationError) or strips only
+ the tag delimiters (leaving 'alert(1)'). Either way, no raw <script>
+ tag is stored in the column name."""
+ try:
+ col = ColumnRef(name="<script>alert(1)</script>")
+ assert "<script>" not in col.name
+ except ValidationError as exc:
+ # nh3 stripped entire element; empty-value guard raises with this
message
+ assert "cannot be empty" in str(exc) # noqa: PT017
Review Comment:
Good catch. Fixed in 65f73aade5: added `assert col.name` to the success path
so a future nh3 regression that returns "" without raising cannot silently pass
the test. The except clause already correctly checks for the "cannot be empty"
message from `sanitize_user_input`'s post-sanitization emptiness guard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
