fix(rls): reject empty or whitespace-only RLS clauses#41297
Conversation
The RLS clause field was validated only for presence and type, so an empty or whitespace-only string could be persisted. As a base filter, an empty clause produces a non-restrictive predicate, silently disabling the control. Add a non-blank validator to the clause field on both the create and update schemas so empty/whitespace-only clauses are rejected. Adds unit tests covering blank/whitespace rejection, valid clauses, and an update that omits the clause. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dosubot
Bot
added
the
authentication:row-level-security
Code Review Agent Run #c55543Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| from superset.row_level_security.schemas import RLSPostSchema, RLSPutSchema | ||
|
|
||
|
|
||
| def _post_payload(**overrides): |
There was a problem hiding this comment.
Suggestion: Add full type annotations to _post_payload, including a typed **overrides parameter and an explicit return type. [custom_rule]
Severity Level: Minor
Why it matters? 🤔
This is a newly added Python helper function and it has no type hints for either
the variadic parameter overrides or the return value. The rule requires new
Python code to be fully typed, so this is a real violation.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** tests/unit_tests/row_level_security/schema_test.py
**Line:** 25:25
**Comment:**
*Custom Rule: Add full type annotations to `_post_payload`, including a typed `**overrides` parameter and an explicit return type.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
The suggestion to add type annotations to the from typing import Any, Dict
def _post_payload(**overrides: Any) -> Dict[str, Any]:
payload = {
"name": "rule",
"filter_type": "Regular",
"tables": [1],
"roles": [1],
"clause": "client_id = 9",
}
payload.update(overrides)
return payloadThere are no other comments on this pull request to address. Would you like me to check for any other potential improvements in this file? tests/unit_tests/row_level_security/schema_test.py |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #41297 +/- ##
==========================================
- Coverage 64.34% 64.34% -0.01%
==========================================
Files 2653 2653
Lines 144952 144955 +3
Branches 33433 33434 +1
==========================================
- Hits 93273 93268 -5
- Misses 49995 50000 +5
- Partials 1684 1687 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
SUMMARY
The RLS
clausefield was validated only for presence and type (required=True/allow_none=False), which does not reject an empty string and has no non-blank constraint. An empty or whitespace-only clause could be persisted; as a base filter (designed to always restrict) an empty clause produces a predicate that constrains nothing, silently disabling the control.This adds a non-blank validator to the
clausefield on both the create (RLSPostSchema) and update (RLSPutSchema) schemas, rejecting empty and whitespace-only clauses on both paths.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — schema validation behavior.
TESTING INSTRUCTIONS
Unit tests added in
tests/unit_tests/row_level_security/schema_test.py:clauseremains valid.Run:
pytest tests/unit_tests/row_level_security/schema_test.pyADDITIONAL INFORMATION