codeant-ai-for-open-source[bot] commented on code in PR #41133:
URL: https://github.com/apache/superset/pull/41133#discussion_r3593629758
##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx:
##########
@@ -198,6 +245,28 @@ export const useDownloadMenuItems = (
];
const exportMenuItems: MenuItem[] = [
+ ...(userCanExport
+ ? [
+ {
+ key: 'export-xlsx',
+ label: t('Export Data to Excel'),
+ onClick: () => onExportXlsx('data'),
+ },
+ // Image export renders charts through the headless webdriver, so
only
+ // offer it where that infrastructure is available (same signal as
the
+ // PDF/PNG image downloads above); otherwise non-table charts would
+ // silently come back empty.
+ ...(isWebDriverScreenshotEnabled
+ ? [
+ {
+ key: 'export-xlsx-images',
+ label: t('Export Images to Excel'),
+ onClick: () => onExportXlsx('images'),
+ },
+ ]
+ : []),
Review Comment:
**Suggestion:** The new image-mode Excel action is gated only by webdriver
feature flags and ignores the existing `canExportImage` permission signal, so
users who are explicitly blocked from image export can still trigger
`mode="images"` and receive chart images via Excel. Gate this menu item with
the same image-export permission logic used for PDF/PNG (and keep backend
authorization aligned) to avoid a permission bypass. [security]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
❌ Dashboard image-export restriction bypassed via Excel images mode.
⚠️ Users without can_export_image can email chart screenshots.
⚠️ Download menu shows image export to unauthorized users.
⚠️ Backend export_xlsx endpoint ignores granular image permission.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. When a dashboard is rendered, the header component `Header/index.tsx:96`
calls
`useHeaderActionsMenu` from
`superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx:41-74`,
passing `userCanExport` (from `HeaderDropdownProps` in
`Header/types.ts:52-56`) and
receiving `canExportImage` from `usePermissions()`
(`superset-frontend/src/hooks/usePermissions.ts:43-52`).
2. For a role where `can_export` on `Dashboard` is granted but
`can_export_image` on
`Superset` is not, `usePermissions` computes `canExportImage` as `false`
while
`userCanExport` remains `true` (`useHeaderActionsDropdownMenu.tsx:57-58`),
so screenshot
items are disabled via `imageDisabled = canExportImage === false`
(`DownloadMenuItems/index.tsx:203`) but this flag is not used for Excel
image export.
3. With webdriver screenshot flags enabled (`isWebDriverScreenshotEnabled` at
`DownloadMenuItems/index.tsx:87-89`) and Excel export configured server-side,
`useDownloadMenuItems` builds `exportMenuItems`
(`DownloadMenuItems/index.tsx:247-269`),
always adding the `export-xlsx-images` menu item when `userCanExport` is
truthy and
`isWebDriverScreenshotEnabled` is true, regardless of `canExportImage`; the
item’s
`onClick` calls `onExportXlsx('images')`
(`DownloadMenuItems/index.tsx:170-201`).
4. Clicking “Export Images to Excel” in the dashboard Download menu
(`DownloadMenuItems.test.tsx:50-63` shows this label wired) issues `POST
/api/v1/dashboard/<pk>/export_xlsx/` with `mode: 'images'`
(`DownloadMenuItems/index.tsx:172-175`), which is authorized by the backend
using only the
dashboard `export` method permission (`superset/dashboards/api.py:306-313`
and
`1597-1605`), allowing a user explicitly denied `can_export_image` to
receive chart images
embedded in an Excel workbook via the async export task
(`superset/tasks/export_dashboard_excel.py:20-32`).
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1f77c76b245c49f58a8cf9a78bbdfe17&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=1f77c76b245c49f58a8cf9a78bbdfe17&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx
**Line:** 259:267
**Comment:**
*Security: The new image-mode Excel action is gated only by webdriver
feature flags and ignores the existing `canExportImage` permission signal, so
users who are explicitly blocked from image export can still trigger
`mode="images"` and receive chart images via Excel. Gate this menu item with
the same image-export permission logic used for PDF/PNG (and keep backend
authorization aligned) to avoid a permission bypass.
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%2F41133&comment_hash=3eb8b275b4e46546ae9a16ef1462b4c180a05461ff5a0f208c6489f53b0920a5&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=3eb8b275b4e46546ae9a16ef1462b4c180a05461ff5a0f208c6489f53b0920a5&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: [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]
