From dee58595992ea16cb363d64ce5910949a34ba81e Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Tue, 23 Jun 2026 11:24:38 -0700 Subject: [PATCH] fix(rls): reject empty or whitespace-only RLS clauses (#41297) Co-authored-by: Amin Ghadersohi Co-authored-by: Claude Fable 5 --- superset/row_level_security/schemas.py | 24 ++++++- .../unit_tests/row_level_security/__init__.py | 16 +++++ .../row_level_security/schema_test.py | 64 +++++++++++++++++++ 3 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 tests/unit_tests/row_level_security/__init__.py create mode 100644 tests/unit_tests/row_level_security/schema_test.py diff --git a/superset/row_level_security/schemas.py b/superset/row_level_security/schemas.py index 3bb0e8337c5..824b4e144fb 100644 --- a/superset/row_level_security/schemas.py +++ b/superset/row_level_security/schemas.py @@ -16,13 +16,25 @@ # under the License. -from marshmallow import fields, Schema +from marshmallow import fields, Schema, ValidationError from marshmallow.validate import Length, OneOf from superset.connectors.sqla.models import RowLevelSecurityFilter from superset.dashboards.schemas import UserSchema from superset.utils.core import RowLevelSecurityFilterType + +def validate_non_blank_clause(value: str) -> None: + """Reject empty or whitespace-only RLS clauses. + + An empty clause produces a non-restrictive predicate, which silently + disables the control when used as a base filter. Require a non-blank clause + on both the create and update paths. + """ + if not value or not value.strip(): + raise ValidationError("clause cannot be empty or whitespace-only.") + + id_description = "Unique if of rls filter" name_description = "Name of rls filter" description_description = "Detailed description" @@ -140,7 +152,10 @@ class RLSPostSchema(Schema): allow_none=True, ) clause = fields.String( - metadata={"description": "clause_description"}, required=True, allow_none=False + metadata={"description": "clause_description"}, + required=True, + allow_none=False, + validate=validate_non_blank_clause, ) @@ -182,5 +197,8 @@ class RLSPutSchema(Schema): allow_none=True, ) clause = fields.String( - metadata={"description": "clause_description"}, required=False, allow_none=False + metadata={"description": "clause_description"}, + required=False, + allow_none=False, + validate=validate_non_blank_clause, ) diff --git a/tests/unit_tests/row_level_security/__init__.py b/tests/unit_tests/row_level_security/__init__.py new file mode 100644 index 00000000000..13a83393a91 --- /dev/null +++ b/tests/unit_tests/row_level_security/__init__.py @@ -0,0 +1,16 @@ +# 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. diff --git a/tests/unit_tests/row_level_security/schema_test.py b/tests/unit_tests/row_level_security/schema_test.py new file mode 100644 index 00000000000..550f3cd7800 --- /dev/null +++ b/tests/unit_tests/row_level_security/schema_test.py @@ -0,0 +1,64 @@ +# 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. +"""Tests for row-level security marshmallow schemas.""" + +from typing import Any + +import pytest +from marshmallow import ValidationError + +from superset.row_level_security.schemas import RLSPostSchema, RLSPutSchema + + +def _post_payload(**overrides: Any) -> dict[str, Any]: + payload: dict[str, Any] = { + "name": "rule", + "filter_type": "Regular", + "tables": [1], + "roles": [1], + "clause": "client_id = 9", + } + payload.update(overrides) + return payload + + +@pytest.mark.parametrize("clause", ["", " ", "\t\n"]) +def test_rls_post_schema_rejects_blank_clause(clause: str) -> None: + """An empty or whitespace-only clause is rejected on create.""" + with pytest.raises(ValidationError) as exc: + RLSPostSchema().load(_post_payload(clause=clause)) + assert "clause" in exc.value.messages + + +def test_rls_post_schema_accepts_non_blank_clause() -> None: + """A non-blank clause is accepted on create.""" + result = RLSPostSchema().load(_post_payload(clause="1 = 0")) + assert result["clause"] == "1 = 0" + + +@pytest.mark.parametrize("clause", ["", " ", "\t\n"]) +def test_rls_put_schema_rejects_blank_clause(clause: str) -> None: + """An empty or whitespace-only clause is rejected on update.""" + with pytest.raises(ValidationError) as exc: + RLSPutSchema().load({"clause": clause}) + assert "clause" in exc.value.messages + + +def test_rls_put_schema_omitted_clause_is_allowed() -> None: + """A partial update that omits clause is still valid.""" + result = RLSPutSchema().load({"name": "new name"}) + assert "clause" not in result