sadpandajoe commented on code in PR #43900:
URL: https://github.com/apache/superset/pull/43900#discussion_r4076978546
##########
docs/developer_docs/extensions/mcp.md:
##########
@@ -462,6 +462,73 @@ async def metrics_guide(ctx: Context) -> str:
"""
```
+## Built-in Dashboard Governance Tools
+
+Superset ships three built-in MCP tools for dashboard governance:
+`manage_dashboard_owners`, `manage_dashboard_roles`, and
+`manage_dashboard_certification`. They exist alongside the generic
+`update_dashboard` tool because a few governance fields are unsafe to expose
+as full-replacement lists to an LLM caller — each tool instead takes
+explicit, narrow operations with its own safety semantics.
+
+### `manage_dashboard_owners`
+
+Adds or removes dashboard owners (the USER-type entries in the dashboard's
+Subject-based `editors` list) via `add_owner_ids`/`remove_owner_ids`, never
+a full-replacement list. This guards against the "empty owners" footgun:
+`update_dashboard` intentionally dropped its `owners` field because a
+full-replacement list has no way to prevent an admin from emptying it
+outright.
+
+- **Owner-removal guard**: a request that would leave the dashboard with
+ zero owners is rejected. To transfer ownership, add the new owner in the
+ same call as removing the last existing one.
+- **Self-removal protection**: a non-admin caller who removes themselves is
+ automatically re-added, mirroring the lockout protection
+ `update_dashboard` already relies on (unless an `EXTRA_EDITORS_RESOLVER`
+ is configured). The response's `warnings` reports when this happens.
+- ROLE- or GROUP-type editors already on the dashboard are left untouched.
+- A no-op request (e.g. "adding" an ID that's already an owner) returns an
+ empty `owners` list rather than the full current set, so the tool can't
+ be used as a disguised directory lookup.
+
+### `manage_dashboard_roles`
+
+Adds or removes dashboard access roles (the ROLE-type entries in the
+dashboard's Subject-based `viewers` list) via
+`add_role_ids`/`remove_role_ids`, never a full-replacement list. This
+guards against silently widening or narrowing who can see a dashboard,
+which is why `update_dashboard` dropped its `roles` field.
+
+- **`ENABLE_VIEWERS` gating**: dashboard access roles only take effect when
Review Comment:
This says disabled `ENABLE_VIEWERS` makes stored role assignments inert, but
dashboard access checks a nonempty `dashboard.viewers` list without that flag.
Adding a role on a published dashboard can therefore immediately restrict users
who previously had datasource-based access. Should this describe the current
behavior (or should the implementation gate enforcement before this ships)?
--
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]
