codeant-ai-for-open-source[bot] commented on code in PR #43336:
URL: https://github.com/apache/superset/pull/43336#discussion_r3814455599
##########
superset/config.py:
##########
@@ -368,6 +368,7 @@ def _try_json_readsha(filepath: str, length: int) -> str |
None:
WTF_CSRF_EXEMPT_LIST = [
"superset.charts.data.api.data",
"superset.dashboards.api.cache_dashboard_screenshot",
+ "superset.dashboards.api.export_xlsx",
Review Comment:
**Suggestion:** Exempting the authenticated `POST` export endpoint from CSRF
protection allows a cross-site request to enqueue an export using a victim's
Superset session whenever cookies are sent cross-site, such as deployments
configured with `SESSION_COOKIE_SAMESITE="None"`. Keep CSRF validation for
cookie-authenticated requests and use a narrowly scoped guest/embedded
authentication mechanism instead of globally exempting this endpoint. [security]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Cross-site origins can trigger authenticated dashboard exports.
- ⚠️ Victim email and worker resources may be consumed.
- ⚠️ CSRF protection is bypassed for a state-changing endpoint.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/config.py
**Line:** 371:371
**Comment:**
*Security: Exempting the authenticated `POST` export endpoint from CSRF
protection allows a cross-site request to enqueue an export using a victim's
Superset session whenever cookies are sent cross-site, such as deployments
configured with `SESSION_COOKIE_SAMESITE="None"`. Keep CSRF validation for
cookie-authenticated requests and use a narrowly scoped guest/embedded
authentication mechanism instead of globally exempting this endpoint.
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%2F43336&comment_hash=f0d2ab67d4d4345e68d44bf04e0e1f0537542aed5af0a9f2d2988c8876ed6a98&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43336&comment_hash=f0d2ab67d4d4345e68d44bf04e0e1f0537542aed5af0a9f2d2988c8876ed6a98&reaction=dislike'>👎</a>
##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx:
##########
@@ -141,49 +154,103 @@ export const useDownloadMenuItems = (
fileName = parsed.parameters.filename;
}
} catch (error) {
- logging.warn('Failed to parse Content-Disposition header:', error);
+ logging.warn("Failed to parse Content-Disposition header:", error);
}
}
// Convert response to blob and trigger download
const blob = await response.blob();
const url = window.URL.createObjectURL(blob);
try {
- const a = document.createElement('a');
+ const a = document.createElement("a");
a.href = url;
a.download = fileName;
- a.style.display = 'none';
+ a.style.display = "none";
document.body.appendChild(a);
a.click();
document.body.removeChild(a);
} finally {
window.URL.revokeObjectURL(url);
}
- addSuccessToast(t('Dashboard exported as example successfully'));
+ addSuccessToast(t("Dashboard exported as example successfully"));
} catch (error) {
logging.error(error);
- addDangerToast(t('Sorry, something went wrong. Try again later.'));
+ addDangerToast(t("Sorry, something went wrong. Try again later."));
}
};
- const onExportXlsx = async (mode: 'data' | 'images') => {
+ const pollExportStatus = (jobId: string, startedAt: number) => {
+ SupersetClient.get({
+ endpoint: `/api/v1/dashboard/export_xlsx/status/${jobId}/`,
+ })
+ .then(({ json }) => {
+ const {
+ status,
+ download_url: downloadUrl,
+ message,
+ } = json as ExportStatusResponse;
+ if (status === "ready") {
+ if (downloadUrl) {
+ window.location.href = downloadUrl;
+ }
+ addSuccessToast(t("Your export is ready and downloading."));
+ return;
+ }
+ if (status === "error") {
+ addDangerToast(
+ message || t("Sorry, something went wrong. Try again later."),
+ );
+ return;
+ }
+ if (Date.now() - startedAt > EXPORT_STATUS_POLL_TIMEOUT_MS) {
+ addDangerToast(
+ t("Your export is taking longer than expected. Try again later."),
+ );
+ return;
+ }
+ setTimeout(
+ () => pollExportStatus(jobId, startedAt),
+ EXPORT_STATUS_POLL_INTERVAL_MS,
+ );
Review Comment:
**Suggestion:** The polling timers are not retained or cancelled when the
hook unmounts or the user navigates away. A pending export can therefore
continue issuing status requests for up to five minutes, and a late successful
response can display a toast or navigate the browser to the download URL after
the originating dashboard has been destroyed. Track the timer and cancel it
during cleanup, and ignore responses after unmount. [missing cleanup]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dashboard navigation leaves polling requests running.
- ⚠️ Late completion can navigate the current page unexpectedly.
- ⚠️ Stale callbacks can display export toasts after teardown.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<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:** 212:215
**Comment:**
*Missing Cleanup: The polling timers are not retained or cancelled when
the hook unmounts or the user navigates away. A pending export can therefore
continue issuing status requests for up to five minutes, and a late successful
response can display a toast or navigate the browser to the download URL after
the originating dashboard has been destroyed. Track the timer and cancel it
during cleanup, and ignore responses after unmount.
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%2F43336&comment_hash=9f3080320a5a7ad6c45440f8d8f6dc40e01fe9ed6377e303f0db3a52870e2f6a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43336&comment_hash=9f3080320a5a7ad6c45440f8d8f6dc40e01fe9ed6377e303f0db3a52870e2f6a&reaction=dislike'>👎</a>
##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx:
##########
@@ -141,49 +154,103 @@ export const useDownloadMenuItems = (
fileName = parsed.parameters.filename;
}
} catch (error) {
- logging.warn('Failed to parse Content-Disposition header:', error);
+ logging.warn("Failed to parse Content-Disposition header:", error);
}
}
// Convert response to blob and trigger download
const blob = await response.blob();
const url = window.URL.createObjectURL(blob);
try {
- const a = document.createElement('a');
+ const a = document.createElement("a");
a.href = url;
a.download = fileName;
- a.style.display = 'none';
+ a.style.display = "none";
document.body.appendChild(a);
a.click();
document.body.removeChild(a);
} finally {
window.URL.revokeObjectURL(url);
}
- addSuccessToast(t('Dashboard exported as example successfully'));
+ addSuccessToast(t("Dashboard exported as example successfully"));
} catch (error) {
logging.error(error);
- addDangerToast(t('Sorry, something went wrong. Try again later.'));
+ addDangerToast(t("Sorry, something went wrong. Try again later."));
}
};
- const onExportXlsx = async (mode: 'data' | 'images') => {
+ const pollExportStatus = (jobId: string, startedAt: number) => {
+ SupersetClient.get({
+ endpoint: `/api/v1/dashboard/export_xlsx/status/${jobId}/`,
+ })
+ .then(({ json }) => {
+ const {
+ status,
+ download_url: downloadUrl,
+ message,
+ } = json as ExportStatusResponse;
+ if (status === "ready") {
+ if (downloadUrl) {
+ window.location.href = downloadUrl;
+ }
+ addSuccessToast(t("Your export is ready and downloading."));
+ return;
+ }
+ if (status === "error") {
+ addDangerToast(
+ message || t("Sorry, something went wrong. Try again later."),
+ );
+ return;
+ }
+ if (Date.now() - startedAt > EXPORT_STATUS_POLL_TIMEOUT_MS) {
+ addDangerToast(
+ t("Your export is taking longer than expected. Try again later."),
+ );
+ return;
+ }
+ setTimeout(
+ () => pollExportStatus(jobId, startedAt),
+ EXPORT_STATUS_POLL_INTERVAL_MS,
+ );
+ })
+ .catch((error) => {
+ // A transient polling failure shouldn't give up the wait -- the export
+ // itself may still succeed -- so keep polling until the timeout.
+ logging.error(error);
+ if (Date.now() - startedAt > EXPORT_STATUS_POLL_TIMEOUT_MS) {
+ addDangerToast(t("Sorry, something went wrong. Try again later."));
+ return;
+ }
+ setTimeout(
+ () => pollExportStatus(jobId, startedAt),
+ EXPORT_STATUS_POLL_INTERVAL_MS,
+ );
+ });
+ };
+
+ const onExportXlsx = async (mode: "data" | "images") => {
try {
const { json } = await SupersetClient.post({
endpoint: `/api/v1/dashboard/${dashboardId}/export_xlsx/`,
jsonPayload: { active_data_mask: buildActiveDataMask(), mode },
});
// The throttle response (an export is already running) returns 202 with
a
// message but no job_id; only a freshly enqueued job carries a job_id.
- if ((json as { job_id?: string })?.job_id) {
+ const jobId = (json as { job_id?: string })?.job_id;
+ if (jobId) {
addSuccessToast(
t(
"Your export is being prepared. You'll receive an email when it's
ready.",
),
);
Review Comment:
**Suggestion:** The pending message explicitly promises that the user will
receive an email, but the newly supported guest and embedded sessions have no
email address and rely exclusively on polling and automatic download. This
gives those users an incorrect delivery expectation; use wording that reflects
the automatic download or select the message based on the session's
notification capability. [logic error]
<details>
<summary><b>Severity Level:</b> Minor 🧹</summary>
```mdx
- ⚠️ Guest users receive an impossible email notification promise.
- ⚠️ Embedded users may wait for an email that never arrives.
- ✅ Polling still enables automatic download completion.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<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:** 242:246
**Comment:**
*Logic Error: The pending message explicitly promises that the user
will receive an email, but the newly supported guest and embedded sessions have
no email address and rely exclusively on polling and automatic download. This
gives those users an incorrect delivery expectation; use wording that reflects
the automatic download or select the message based on the session's
notification capability.
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%2F43336&comment_hash=7d60d2cb08ce969ea52acb982f729328ac236e2d13a96e79a10205450f2b78de&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43336&comment_hash=7d60d2cb08ce969ea52acb982f729328ac236e2d13a96e79a10205450f2b78de&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]
