YuriyKrasilnikov commented on PR #37395:
URL: https://github.com/apache/superset/pull/37395#issuecomment-3792157583

   ### Response to codeant-ai bot suggestions
   
   **1. rls.py:114 - "Logic/security regression"**
   
   The bot's concern is incorrect. The change is intentional and does NOT 
disable guest RLS globally.
   
   **Architecture:**
   - `get_predicates_for_table()` is called only for **underlying tables** in 
virtual dataset SQL (via `apply_rls()`)
   - Guest RLS for the **virtual dataset itself** is applied separately in 
`get_sqla_query()` line 3198 via `get_sqla_row_level_filters()` which always 
calls `_get_sqla_row_level_filters_internal(include_guest_rls=True)`
   
   **Flow:**
   1. `get_from_clause()` → `apply_rls()` → underlying tables get regular RLS 
only (no guest RLS)
   2. `get_sqla_query()` → `get_sqla_row_level_filters()` → virtual dataset 
gets guest RLS
   
   This prevents **double application** of guest RLS (the bug described in 
#37359), while ensuring guest RLS is still applied exactly once at the correct 
level.
   
   ---
   
   **2-4. test_double_rls_virtual_dataset.py - "with (patch(...),) tuple 
syntax"**
   
   The bot is incorrect. The syntax `with (patch(...), patch(...)):` is valid 
Python 3.10+ syntax per [PEP 617](https://peps.python.org/pep-0617/). Superset 
CI uses Python 3.10/3.11 where this is fully supported.


-- 
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]

Reply via email to