bito-code-review[bot] commented on code in PR #38452:
URL: https://github.com/apache/superset/pull/38452#discussion_r2932891903


##########
superset/models/helpers.py:
##########
@@ -184,24 +190,121 @@ class CachedTimeOffset(TypedDict):
 }
 
 
+def _apply_rls_with_cycle_detection(
+    database: Database,
+    catalog: str | None,
+    default_schema: str,
+    parsed_statement: SQLStatement,
+) -> None:
+    """
+    Apply RLS to a statement while detecting and preventing circular policies.
+    """
+    table_ids = RLS_IN_PROGRESS_TABLE_IDS.get()
+    # Max depth safety brake to prevent DoS
+    if len(table_ids) > 10:
+        raise SupersetSecurityException(
+            SupersetError(
+                
error_type=SupersetErrorType.FAILED_FETCHING_DATASOURCE_INFO_ERROR,
+                message=_("Maximum RLS nesting depth exceeded."),
+                level=ErrorLevel.ERROR,
+            )
+        )
+
+    # For cycle detection, we add ALL tables from the current statement
+    # to the stack before applying RLS.
+    all_dataset_ids = set()
+    for table in parsed_statement.tables:
+        qualified_table = table.qualify(
+            catalog=catalog or database.get_default_catalog(),
+            schema=default_schema,
+        )
+        from superset.connectors.sqla.models import SqlaTable
+
+        try:
+            dataset = (
+                db.session.query(SqlaTable)
+                .filter(
+                    SqlaTable.database_id == database.id,
+                    SqlaTable.catalog == qualified_table.catalog,
+                    SqlaTable.schema == qualified_table.schema,
+                    SqlaTable.table_name == qualified_table.table,
+                )
+                .one_or_none()
+            )
+            if dataset:
+                if dataset.id in table_ids:
+                    raise SupersetSecurityException(
+                        SupersetError(
+                            
error_type=SupersetErrorType.FAILED_FETCHING_DATASOURCE_INFO_ERROR,
+                            message=_(
+                                "Circular RLS policy detected for table: 
%(table)s",
+                                table=dataset.table_name,
+                            ),
+                            level=ErrorLevel.ERROR,
+                        )
+                    )
+                all_dataset_ids.add(dataset.id)
+        except Exception:  # pylint: disable=broad-except

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Avoid blind exception catching</b></div>
   <div id="fix">
   
   Replace broad `Exception` catch with specific exception types (e.g., 
`(AttributeError, KeyError, TypeError)`) to avoid masking unexpected errors.
   </div>
   
   
   <details>
   <summary>
   <b>Code suggestion</b>
   </summary>
   <blockquote>Check the AI-generated fix before applying</blockquote>
   <div id="code">
   
   
   ````suggestion
                   all_dataset_ids.add(dataset.id)
           except (AttributeError, KeyError, TypeError, ValueError):  # pylint: 
disable=broad-except
   ````
   
   </div>
   </details>
   
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #ac6346</i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a 
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



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