EnxDev commented on code in PR #45231:
URL: https://github.com/apache/superset/pull/45231#discussion_r4255951546


##########
superset/mcp_service/dataset_scope.py:
##########
@@ -211,6 +234,79 @@ def get_dataset_scope() -> frozenset[UUID] | None:
     )
 
 
+def _resolve_callable_scope(
+    resolver: Callable[[Any], Iterable[str | UUID] | None],
+) -> frozenset[UUID]:
+    """Call a configured ``MCP_DATASET_ROLE_ALLOWLIST`` resolver for this 
request.
+
+    Fails CLOSED -- denies every dataset for this request -- if the resolver
+    raises, returns ``None``, or returns something that cannot be parsed as
+    dataset UUIDs. This is deliberately different from a bare ``None``
+    *config* at the top level, which disables routing constraints entirely:
+    once an operator has configured dynamic routing, a resolution failure for
+    one request (an unresolvable workspace, a transient lookup error) must
+    deny access rather than silently fall back to the unrestricted tool
+    surface for that caller.
+    """
+    user = getattr(g, "user", None)
+    # Log the numeric id rather than the username so a failing tenant can be
+    # traced without writing identity claims to the log.
+    user_id = getattr(user, "id", None)
+    try:
+        result = resolver(user)
+        if result is None:

Review Comment:
   Since `None` from the resolver means deny-all, it has no way to say "this 
tenant isn't scoped". In a deployment where only some workspaces opt in, the 
others would have to enumerate every dataset, and they'd still lose 
`execute_sql` and the other unsupported tools because the scope is never `None`.
   
   Is that intended for the multi-tenant case? If so it deserves a line in the 
docs. Otherwise an explicit sentinel for "unrestricted" would allow it while 
keeping the fail-closed default.



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