bikash-barnwal opened a new pull request, #43179:
URL: https://github.com/apache/superset/pull/43179
### SUMMARY
On an embedded dashboard, **View query** opened a modal with a blank query
and no error, even for an embedded role the operator had granted `can view
query` on `Dashboard`.
The two ends disagreed:
- The menu item is rendered when `canExplore || canViewQuery`, where
`canViewQuery` comes from `findPermission('can_view_query', 'Dashboard',
user.roles)` (`SliceHeaderControls`, `usePermissions.ts:73`). For an embedded
viewer, `user` is populated after the guest-token handshake by `GET
/api/v1/me/roles/`, which returns the guest role's real permission tuples — so
the item appears exactly when the operator granted that permission.
- `ChartDataRestApi._send_chart_response` popped `"query"` from every result
for *any* guest user, unconditionally, with no permission check. The response
was `{"result": [{"language": "sql"}]}` and the modal rendered empty.
This gates the SQL on the same permission that gates the menu item,
mirroring the existing check on the neighbouring export path
(`superset/charts/data/api.py:492`).
Deliberately kept separate from the gate: engine errors and stacktraces stay
redacted for **every** guest regardless of the permission, since they can name
internal tables, schemas or hosts. The guest block is split so that
sanitization is unconditional and only the `query` pop is conditional.
Security posture — the default is unchanged and fail-closed.
`GUEST_ROLE_NAME` defaults to `Public` (`superset/config.py:2901`) and
`('can_view_query', 'Dashboard')` is not in `PUBLIC_ROLE_PERMISSIONS`, so out
of the box a guest still gets no SQL. Exposing it is an explicit operator
decision to grant a view-menu permission to the embedded role, which
`SECURITY.md` lists as a supported deployment choice.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: **View query** on an embedded dashboard opens an empty modal.
After: the generated SQL is shown when the embedded role holds `can view
query` on `Dashboard`; without it the SQL is still withheld.
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/charts/test_chart_data_api.py
pytest tests/integration_tests/charts/data/api_tests.py -k guest
```
New coverage:
- a permitted guest keeps `query` in the payload;
- a permitted guest still gets its `error` sanitized and `stacktrace`
stripped;
- a guest without the permission still has `query` removed (existing tests,
now explicit about the permission state).
Manually: embed a dashboard with a guest token whose role has `can view
query` on `Dashboard`, open a chart's ⋯ menu → **View query**, and confirm the
SQL renders. Remove the permission and confirm the menu item disappears.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #43100
- [ ] 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
Note: the Python tests were not executed locally (no working Superset
virtualenv on the authoring machine) and rely on CI; `ruff format` and `ruff
check` are clean on all changed files.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
