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


##########
superset/mcp_service/chart/tool/update_chart.py:
##########
@@ -956,31 +1154,39 @@ async def update_chart(  # noqa: C901
             # Validate before caching the form_data — same rationale as above.
             if validation_config is not None:
                 with 
event_logger.log_context(action="mcp.update_chart.validation"):
-                    validation_error = _validate_update_against_dataset(
+                    validation_error = _validate_update_against_target(
                         validation_config,
                         preview_or_error,
                         chart,
-                        dataset_id=request.dataset_id,
+                        rebind_id,
+                        rebind_type,
                     )
                 if validation_error is not None:
                     return validation_error
-            elif request.dataset_id is not None:
-                # Dataset-only rebind: verify the target dataset exists before
-                # caching. Skip compile check — no new config to execute.
+            elif rebind_id is not None:
+                # Table-only rebinds preserve legacy existence-only validation.
+                # Semantic transitions must validate the retained query roles.
                 with 
event_logger.log_context(action="mcp.update_chart.validation"):
-                    validation_error = _validate_update_against_dataset(
+                    validation_error = _validate_update_against_target(
                         None,
-                        {},
+                        preview_or_error
+                        if DatasourceType.SEMANTIC_VIEW.value
+                        in (rebind_type, _chart_datasource_type(chart))
+                        else {},

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>CWE-20: Preview rebind validation weakened</b></div>
   <div id="fix">
   
   Lines 1172-1175 pass `{}` for table→table source rebinds, but the deleted 
`-preview_or_error,` line proves the preview path previously passed the merged 
form data, so `_validate_adhoc_filter_columns` (compile.py:443) no longer 
checks retained adhoc filters against the target dataset before caching. The 
persist path's gap (line 1132) is now replicated here. If intended, reword the 
comment; otherwise pass `preview_or_error` unconditionally. 
([CWE-20](https://cwe.mitre.org/data/definitions/20.html))
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #36f325</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