rusackas commented on code in PR #43900:
URL: https://github.com/apache/superset/pull/43900#discussion_r4077980374
##########
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:
Good catch. `raise_for_access` checks `dashboard.viewers` directly with no
`ENABLE_VIEWERS` gate, so a role added here restricts access right away even
with the flag off (there's a test proving it:
`test_raise_for_access_dashboard_viewers_override_implicit_access`). Fixed the
doc to describe that instead of "no live effect."
--
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]
