You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/09/04 20:20:38 UTC
[PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
rusackas opened a new pull request, #43900:
URL: https://github.com/apache/superset/pull/43900
### SUMMARY
#41606 added the `manage_dashboard_owners`, `manage_dashboard_roles`, and `manage_dashboard_certification` MCP tools, but their safety semantics never made it into the docs. This adds a section to the MCP tools reference covering the owner-removal guard on `manage_dashboard_owners`, the `ENABLE_VIEWERS` gating on `manage_dashboard_roles`, and the `certified_by`/`certification_details` clear-vs-unchanged convention on `manage_dashboard_certification`.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A, docs only.
### TESTING INSTRUCTIONS
Docs-only change. Ran pre-commit on the touched file. Once CI kicks off there should be a Netlify preview link to check the rendered page.
### ADDITIONAL INFORMATION
- [ ] Has associated issue: refs #41606
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #43900:
URL: https://github.com/apache/superset/pull/43900#discussion_r4151107268
##########
docs/developer_docs/extensions/mcp.md:
##########
@@ -462,6 +462,76 @@ 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.
Review Comment:
An editor can still retrieve the full current owner or role ID list without changing anything by requesting removal of an unassigned ID: both tools include the current list in that error response. Should this avoid promising that the tools cannot be used as directory lookups unless those error paths also withhold the lists?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43900:
URL: https://github.com/apache/superset/pull/43900#issuecomment-5950869347
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43900:
URL: https://github.com/apache/superset/pull/43900#issuecomment-5954522874
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #43900:
URL: https://github.com/apache/superset/pull/43900#discussion_r4078011568
##########
docs/developer_docs/extensions/mcp.md:
##########
@@ -462,6 +462,74 @@ 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` reporting**: the response reports the flag's current
+ state via `viewers_enabled` and `warnings` notes when it's disabled β but
+ that warning is informational only. `raise_for_access` enforces a
+ nonempty `viewers` list on a published dashboard regardless of the flag,
+ so a role added here restricts access immediately even with
+ `ENABLE_VIEWERS` off. An empty roles list simply means "no role
+ restriction", i.e. normal permissions apply.
Review Comment:
**Suggestion:** When USER- or GROUP-type viewers remain, an empty role list does not restore normal permissions; the existing viewer subjects still restrict dashboard access.
**Assessment:** π `Major` Β· π `Occurrence: Sometimes` Β· π·οΈ `Api mismatch`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d8f416ba36464434ad8e2fe3125dfa8e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d8f416ba36464434ad8e2fe3125dfa8e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docs/developer_docs/extensions/mcp.md
**Line:** 508:509
**Comment:**
*Api Mismatch: When USER- or GROUP-type viewers remain, an empty role list does not restore normal permissions; the existing viewer subjects still restrict dashboard access.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43900&comment_hash=ed442c938f983a0ac636f6446eb723a1c83e9ae474d4c5c8fc50798c77ad8ee5&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43900&comment_hash=ed442c938f983a0ac636f6446eb723a1c83e9ae474d4c5c8fc50798c77ad8ee5&reaction=dislike'>π</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #43900:
URL: https://github.com/apache/superset/pull/43900#discussion_r4163173276
##########
superset/mcp_service/dashboard/tool/manage_dashboard_roles.py:
##########
@@ -171,12 +171,13 @@ def manage_dashboard_roles(
Add or remove dashboard access roles with explicit operations.
Dashboard access roles restrict who can view a dashboard to members of
- the listed roles, on top of normal Superset permissions. An empty roles
- list means "no role restriction" β the dashboard is visible per standard
- permissions instead. This only takes effect when the ``ENABLE_VIEWERS``
- feature flag is enabled; the response's ``viewers_enabled`` field
- reports whether it is, and ``warnings`` notes when a change was applied
- but has no live effect.
+ the listed roles, on top of normal Superset permissions. Removing every
+ role only restores standard permissions if no USER- or GROUP-type
+ viewers remain on the dashboard β ``raise_for_access`` restricts access
+ whenever the ``viewers`` list is nonempty, regardless of subject type.
+ This only takes effect when the ``ENABLE_VIEWERS`` feature flag is
+ enabled; the response's ``viewers_enabled`` field reports whether it is,
+ and ``warnings`` notes when a change was applied but has no live effect.
Review Comment:
Still says the flag gates enforcement, contradicting the sentence above and mcp.md: `raise_for_access` restricts on any nonempty `viewers` with `ENABLE_VIEWERS` off. This is the LLM-facing tool description, and the flag-off warning at L224-228 makes the same "no effect" claim.
```suggestion
This applies regardless of the ``ENABLE_VIEWERS`` feature flag; the
response's ``viewers_enabled`` field reports the flag's state, and
``warnings`` notes when it is disabled.
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #43900:
URL: https://github.com/apache/superset/pull/43900#discussion_r4159004083
##########
docs/developer_docs/extensions/mcp.md:
##########
@@ -462,6 +462,76 @@ 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.
Review Comment:
Good catch, you are right. Both tools echo the current ID list when you ask to remove an unassigned ID. I dropped the "disguised directory lookup" claim from the docs and now say that error path exposes the list.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #43900:
URL: https://github.com/apache/superset/pull/43900#issuecomment-5546040227
### <span aria-hidden="true">β
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">π¨</span> Latest commit | 40b7bd19a2d6b537520f6f6fe61be570d2e3ff58 |
|<span aria-hidden="true">π</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a9b281909626300085c043a |
|<span aria-hidden="true">π</span> Deploy Preview | [https://deploy-preview-43900--superset-docs-preview.netlify.app](https://deploy-preview-43900--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">π±</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">π€</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcxLCsMwDADRu2hdEeVTEfcGPYZs2aTQ2EYOpKX07vFuYOD9oFrZ6wEPgBt4kxy23un1GVIfWA2XkYkxx6gNtYSGKcqBe6goqqjSNl_EFMuZo2FXNNZ3-T61OyzOT-voyPHEMxGt90DLLPC_AA) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-43900--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43900:
URL: https://github.com/apache/superset/pull/43900#issuecomment-5546008459
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #43900:
URL: https://github.com/apache/superset/pull/43900
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #43900:
URL: https://github.com/apache/superset/pull/43900#discussion_r4081009754
##########
docs/developer_docs/extensions/mcp.md:
##########
@@ -462,6 +462,74 @@ 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` reporting**: the response reports the flag's current
+ state via `viewers_enabled` and `warnings` notes when it's disabled β but
+ that warning is informational only. `raise_for_access` enforces a
+ nonempty `viewers` list on a published dashboard regardless of the flag,
+ so a role added here restricts access immediately even with
+ `ENABLE_VIEWERS` off. An empty roles list simply means "no role
+ restriction", i.e. normal permissions apply.
Review Comment:
Good catch. Emptying the role list only clears ROLE-type viewers, so leftover USER/GROUP viewers still restrict access via `raise_for_access`. Docs and the tool docstring now say that.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #43900:
URL: https://github.com/apache/superset/pull/43900#issuecomment-6109618266
## CodeAnt PR Risk: Low Risk
- The PR appears safe to merge; the documentation clarifies that remaining USER- or GROUP-type viewers can still restrict dashboard access.
- The role-tool documentation now states that access restrictions apply regardless of the ENABLE_VIEWERS flag.
**Assessed commit:** `06c5fdfa242e`
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43900:
URL: https://github.com/apache/superset/pull/43900#issuecomment-5795038877
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43900:
URL: https://github.com/apache/superset/pull/43900#issuecomment-5941056677
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43900:
URL: https://github.com/apache/superset/pull/43900#issuecomment-5784921230
<!-- Bito Reply -->
The documentation accurately reflects the current implementation described in the diff. The `manage_dashboard_roles` tool stores the roles list regardless of the `ENABLE_VIEWERS` flag, but the dashboard access check logic (which is external to this tool) relies on the flag to determine whether to enforce those restrictions. If the flag is disabled, the stored roles are indeed inert, but the dashboard access check logic may still be evaluating the presence of roles if it is not also gated by the same flag. It is recommended to ensure that the dashboard access check logic is consistently gated by `ENABLE_VIEWERS` to prevent unintended access restrictions when the flag is disabled.
**docs/developer_docs/extensions/mcp.md**
```
+- **`ENABLE_VIEWERS` gating**: dashboard access roles only take effect when
+ the `ENABLE_VIEWERS` feature flag is enabled. The response always reports
+ the flag's current state via `viewers_enabled`, and `warnings` notes when
+ a change was applied but has no live effect (the roles list is still
+ stored either way β an empty roles list simply means "no role
+ restriction", i.e. normal permissions apply).
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #43900:
URL: https://github.com/apache/superset/pull/43900#discussion_r4163858276
##########
superset/mcp_service/dashboard/tool/manage_dashboard_roles.py:
##########
@@ -171,12 +171,13 @@ def manage_dashboard_roles(
Add or remove dashboard access roles with explicit operations.
Dashboard access roles restrict who can view a dashboard to members of
- the listed roles, on top of normal Superset permissions. An empty roles
- list means "no role restriction" β the dashboard is visible per standard
- permissions instead. This only takes effect when the ``ENABLE_VIEWERS``
- feature flag is enabled; the response's ``viewers_enabled`` field
- reports whether it is, and ``warnings`` notes when a change was applied
- but has no live effect.
+ the listed roles, on top of normal Superset permissions. Removing every
+ role only restores standard permissions if no USER- or GROUP-type
+ viewers remain on the dashboard β ``raise_for_access`` restricts access
+ whenever the ``viewers`` list is nonempty, regardless of subject type.
+ This only takes effect when the ``ENABLE_VIEWERS`` feature flag is
+ enabled; the response's ``viewers_enabled`` field reports whether it is,
+ and ``warnings`` notes when a change was applied but has no live effect.
Review Comment:
@aminghadersohi good catch. Fixed the docstring and the flag-off warning to say role changes apply regardless of `ENABLE_VIEWERS`, and dropped the "disguised directory lookup" line in both the roles and owners tool docstrings.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #43900:
URL: https://github.com/apache/superset/pull/43900#issuecomment-5951178360
@aminghadersohi fixed both: the `viewers_enabled` field description in `schemas.py` and the tool list line in `app.py` now say role changes apply regardless of `ENABLE_VIEWERS`.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): document dashboard owner/role/certification governance tools [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #43900:
URL: https://github.com/apache/superset/pull/43900#issuecomment-5793219510
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/43900?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 65.35%. Comparing base ([`0636ac4`](https://app.codecov.io/gh/apache/superset/commit/0636ac4b1148463e479e41b052aadd04ac0cb472?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`3d41f25`](https://app.codecov.io/gh/apache/superset/commit/3d41f259135bf999aedf4734699211ea17686818?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 531 commits behind head on master.
> :exclamation: There is a different number of reports uploaded between BASE (0636ac4) and HEAD (3d41f25). Click for more details.
>
> <details><summary>HEAD has 62 uploads less than BASE</summary>
>
>| Flag | BASE (0636ac4) | HEAD (3d41f25) |
>|------|------|------|
>|python|42|4|
>|presto|7|1|
>|hive|7|1|
>|mysql|6|1|
>|postgres|8|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #43900 +/- ##
===========================================
- Coverage 79.48% 65.35% -14.14%
===========================================
Files 2896 2930 +34
Lines 168323 175995 +7672
Branches 39059 40519 +1460
===========================================
- Hits 133790 115016 -18774
- Misses 32034 58470 +26436
- Partials 2499 2509 +10
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/43900/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/43900/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `36.86% <ΓΈ> (-0.92%)` | :arrow_down: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/43900/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.02% <ΓΈ> (-1.47%)` | :arrow_down: |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/43900/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.02% <ΓΈ> (-1.50%)` | :arrow_down: |
| [presto](https://app.codecov.io/gh/apache/superset/pull/43900/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `38.79% <ΓΈ> (-0.88%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/43900/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.24% <ΓΈ> (-27.64%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/43900/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/43900/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/43900?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org